Skip to content

fix: guard link recognizer against stale/detached node in _tapNodeLink - #2753

Open
mtallenca wants to merge 1 commit into
singerdmx:masterfrom
mtallenca:fix/stale-link-node-crash
Open

fix: guard link recognizer against stale/detached node in _tapNodeLink#2753
mtallenca wants to merge 1 commit into
singerdmx:masterfrom
mtallenca:fix/stale-link-node-crash

Conversation

@mtallenca

Copy link
Copy Markdown
Contributor

Description

_TextLineState caches per-node link gesture recognizers in _linkRecognizers (Map<Node, GestureRecognizer>), and the recognizer's callback captures a specific segment node:

_linkRecognizers[segment] = TapGestureRecognizer()
  ..onTap = () => _tapNodeLink(segment);

That map is only cleared on a readOnly toggle (didUpdateWidget), a meta/ctrl key change (_pressedKeysChanged), or disposenever when the line's document content changes. So the cached recognizer (and the node it captured) can outlive a document mutation/re-render.

When the gesture finally fires, _tapNodeLink/_longPressLink re-read the node's link attribute and force-unwrap it:

void _tapNodeLink(Node node) {
  final link = node.style.attributes[Attribute.link.key]!.value; // throws if null
  _tapLink(link);
}

If the document mutated between recognizer creation and the tap being swept by the gesture arena, the captured node can be detached or no longer carry a link attribute, so attributes[Attribute.link.key] is null and the ! throws.

We see this crash in production (Crashlytics, flutter_quill 11.5.1, Android) on a read-only document with many links that is re-rendered while the user navigates — a queued tap is delivered during GestureArenaManager.sweep against a now-stale recognizer:

Fatal Exception: Null check operator used on a null value
0  _TextLineState._tapNodeLink (text_line.dart:670)
1  _TextLineState._getRecognizer.<fn> (text_line.dart:656)
2  GestureRecognizer.invokeCallback (recognizer.dart:345)
3  TapGestureRecognizer.handleTapUp (tap.dart:758)
4  BaseTapGestureRecognizer._checkUp (tap.dart:383)
5  BaseTapGestureRecognizer.acceptGesture (tap.dart:353)
6  GestureArenaManager.sweep (arena.dart:173)

Fix

Read the link attribute defensively (?.value) in both _tapNodeLink and _longPressLink instead of force-unwrapping. _tapLink already no-ops on a null link; _longPressLink gets an early return. When the captured node is stale, the gesture simply does nothing instead of crashing.

Notes

This is a defensive guard against a timing/staleness window (stale cached recognizer fired during arena sweep after a document mutation), which is awkward to reproduce deterministically in a widget test. Happy to add a regression test if a maintainer can point me at the preferred way to drive a cached recognizer against a mutated node.

_TextLineState caches link GestureRecognizers in _linkRecognizers keyed
by Node, but that map is only cleared on readOnly-toggle, meta-key change,
or dispose -- never when the line content changes. The onTap/onLongPress
closure captures a specific segment node and re-reads its link attribute
when the gesture fires. If the document mutates (e.g. a note with many
links re-rendered while navigating), the captured node can be detached or
no longer carry a link attribute by the time the arena sweeps the tap,
making node.style.attributes[Attribute.link.key]! throw a null-check crash.

Read the link attribute defensively (?.value) in both _tapNodeLink and
_longPressLink; _tapLink already no-ops on null.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant